-
Notifications
You must be signed in to change notification settings - Fork 71
@W-21200034: Add scaffolding to override some config entries at runtime #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds infrastructure for runtime configuration overrides via MCP site settings, preparing for a future REST API that will allow server-side configuration management. The changes introduce an OverridableConfig class that separates runtime-overridable settings from static Config settings, implements caching for MCP site settings with configurable TTL, and updates all tool methods to use the override mechanism.
Changes:
- Extracted overridable configuration logic from
Configinto a newOverridableConfigclass that supports runtime overrides - Added caching infrastructure using
ExpiringMapto minimize API calls when fetching MCP site settings (placeholder implementation returns empty settings until REST API is available) - Updated
RestApiArgstype to support optional logging via discriminated union, enabling config fetching during server initialization - Modified all tool methods to call
getConfigWithOverrides()and use the overridable bounded context and limits
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| types/process-env.d.ts | Added environment variables for MCP site settings feature flag and cache interval |
| src/utils/types.ts | Added DistributiveOmit utility type for properly handling discriminated unions |
| src/utils/mcpSiteSettings.ts | Core logic for fetching, caching, and applying MCP site settings overrides |
| src/utils/mcpSiteSettings.test.ts | Tests for MCP site settings functionality including caching behavior |
| src/overridableConfig.ts | New class containing all runtime-overridable configuration logic extracted from Config |
| src/overridableConfig.test.ts | Comprehensive tests for OverridableConfig class |
| src/sdks/tableau/types/mcpSiteSettings.ts | Type definition for MCP site settings as string-to-string map |
| src/sdks/tableau/restApi.ts | Added placeholder siteMethods.getMcpSettings() that returns empty settings |
| src/config.ts | Removed overridable settings; added new MCP site settings configuration properties |
| src/config.test.ts | Updated tests after extracting overridable config logic |
| src/restApiInstance.ts | Refactored to use RestApiArgs type with discriminated union for optional logging |
| src/server.ts | Made _getToolsToRegister async to support fetching config with overrides |
| src/tools/resourceAccessChecker.ts | Updated to use async getter methods that fetch overridable config |
| src/tools/workbooks/listWorkbooks.ts | Updated to fetch and use config with overrides |
| src/tools/workbooks/getWorkbook.ts | Updated to fetch and use config with overrides |
| src/tools/views/listViews.ts | Updated to fetch and use config with overrides |
| src/tools/views/getViewImage.ts | Updated to construct restApiArgs consistently |
| src/tools/views/getViewData.ts | Updated to construct restApiArgs consistently |
| src/tools/queryDatasource/queryDatasource.ts | Updated to fetch and use config with overrides |
| src/tools/pulse/*.ts | Updated all Pulse tools to fetch and use config with overrides |
| src/tools/listDatasources/listDatasources.ts | Updated to fetch and use config with overrides |
| src/tools/getDatasourceMetadata/getDatasourceMetadata.ts | Updated to fetch and use config with overrides |
| src/tools/contentExploration/searchContent.ts | Updated to fetch and use config with overrides |
| src/tools/contentExploration/searchContentUtils.ts | Updated BoundedContext import |
| src/tools/pulse/constrainPulseMetrics.ts | Updated BoundedContext import |
| src/tools/pulse/constrainPulseDefinitions.ts | Updated BoundedContext import |
| src/scripts/createClaudeMcpBundleManifest.ts | Added new environment variables to manifest |
| package.json | Version bump to 1.14.7 |
| package-lock.json | Version bump to 1.14.7 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
These changes lay the groundwork for @W-21200059: Add Get/Update MCP site settings REST APIs.
Once the "Get MCP Site Settings" REST API is available, we can call it to get the actual site settings. Currently, the
ENABLE_MCP_SITE_SETTINGSfeature is disabled but can be enabled for testing. When enabled, the method that retrieves the MCP site settings returns a placeholder empty object (implying no overrides).